Misc tidying #837
Misc tidying #837
Conversation
| @@ -283,15 +282,18 @@ async function handleChatToGPT(req: OpenAiChatRequest, res: Response) { | |||
| } | |||
|
|
|||
| let updatedChatHistory = levelResult.chatHistory; | |||
| totalSentEmails.push(...levelResult.chatResponse.sentEmails); | |||
|
|
|||
| const totalSentEmails: EmailInfo[] = [ | |||
There was a problem hiding this comment.
before, this was being instantiated then mutated once to add on the new emails sent as a result of the message. Now we just instantiate it including the new emails
| console.debug(`QA model response: ${response.text}`); | ||
| const result: ChatAnswer = { | ||
| reply: response.text, | ||
| questionAnswered: true, |
There was a problem hiding this comment.
We weren't using this questionAnswered value anywhere.
| } else { | ||
| console.error('No arguments provided to askQuestion function'); | ||
| return { reply: "Reply with 'I don't know what to ask'" }; |
There was a problem hiding this comment.
there was no reason to wrap this in an object either
pmarsh-scottlogic
requested review from
chriswilty,
AAloshine-scottlogic and
dhinrichs-scottlogic
|
Thanks @AAloshine-scottlogic I'll leave the PR up until end of Monday, say, so folk can weigh in if they want to |
backend/src/openai.ts
Outdated
| chatHistory: ChatMessage[], | ||
| gptModel: CHAT_MODELS | ||
| ): ChatCompletionMessageParam[] { | ||
| // take only completions to send to model | ||
| const completions = chatHistory.reduce<ChatCompletionMessageParam[]>( |
There was a problem hiding this comment.
Confused as to why this is a reduce, rather than a simpler filter.
There was a problem hiding this comment.
yeah good spot, I'll change that
There was a problem hiding this comment.
We need to get all the completions, but not every chatMessage has a completion. So i've swapped it for a map then a filter:
const completions = chatHistory
.map((chatMessage) =>
'completion' in chatMessage ? chatMessage.completion : null
)
.filter(
(completion) => completion !== null
) as ChatCompletionMessageParam[];I'm sure there's a nicer way though. I'm sure you've helped me with this particular scenario before as well, but I can't find where
There was a problem hiding this comment.
Oh, this rings a bell now... It was the awkward typings that meant the reduce seemed like the least verbose solution. I seem to recall you couldn't simply filter then map, as then you needed the type check in both places. Oh well.
frontend/src/components/HandbookOverlay/Pages/HandbookAttacks.test.tsx
Outdated
Show resolved
Hide resolved
frontend/src/components/HandbookOverlay/Pages/HandbookAttacks.tsx
Outdated
Show resolved
Hide resolved
frontend/src/components/HandbookOverlay/Pages/HandbookGlossary.test.tsx
Outdated
Show resolved
Hide resolved
frontend/src/components/HandbookOverlay/Pages/HandbookGlossary.tsx
Outdated
Show resolved
Hide resolved
Description
Some low-hanging fruit tidying that I did while scouring the codebase in order to write docs.
Notes
queryPromptEvaluationModeltoevaluatePrompt. It now returns a boolean instead of wrapping it in an object for no good reason.queryDocumentsnow just returns a string rather than wrapping it in a object for no good reason.Checklist
Have you done the following?